Fixes for dataclass_array_container#116
Conversation
|
|
alexfikl
changed the title
alexfikl
changed the title
inducer
left a comment
inducer
left a comment
There was a problem hiding this comment.
Thanks! Not loving either change, unfortunately. Let's discuss.
alexfikl
force-pushed
the
dataclass-container-strings
branch
from
17dbce4 to
eae95b5
Compare
arraycontext/container/__init__.py
Outdated
| """ | ||
| assert isinstance(cls, type), \ | ||
| f"must pass a type, not an instance: '{cls!r}'" | ||
| assert hasattr(cls, "__mro__"), "'cls' has no attribute '__mro__': " |
There was a problem hiding this comment.
Actually, is this __mro__ check needed? Everything that is an instance of type should have an __mro__ defined, right?
There was a problem hiding this comment.
I don't think it's needed.
Just to check, isinstance(..., type) does seem to successfully rule out the typing stuff:
>>> from typing import Sequence
>>> isinstance(Sequence, type)
False
There was a problem hiding this comment.
Thanks for checking! Removed in 1a28536.
alexfikl
force-pushed
the
dataclass-container-strings
branch
from
eae95b5 to
e1d0dc6
Compare
inducer
left a comment
inducer
left a comment
There was a problem hiding this comment.
Thanks! LGTM with the MRO check removed.
arraycontext/container/__init__.py
Outdated
| """ | ||
| assert isinstance(cls, type), \ | ||
| f"must pass a type, not an instance: '{cls!r}'" | ||
| assert hasattr(cls, "__mro__"), "'cls' has no attribute '__mro__': " |
There was a problem hiding this comment.
I don't think it's needed.
Just to check, isinstance(..., type) does seem to successfully rule out the typing stuff:
>>> from typing import Sequence
>>> isinstance(Sequence, type)
False
|
Thanks! |
2 participants
There's a couple of random fixes to the dataclass helper in here:
Add an assert in
is_array_container_typethatisinstance(cls, type). If a dataclass has anOptionalorUnionfield (which aren't classes),is_array_container_typewould raise someAttributeErroranyway, but with a more cryptic message.Added some checks in
dataclass_array_containerto give better error messages when it encounters unsuported field types: withinit=False, string annotations or classes wrapped inOptional(or equivalent).